Give user a link to create PR after push by JulienTant · Pull Request #4716 · go-gitea/gitea

JulienTant · 2018-08-15T10:02:10Z

New Feature

This PR offers a link to create new pull request when we push to a branch which is not the default branch.
If a PR already exists, it just give the link to the existing PR.

Output of a push when no active PR exists:

[branch2/test be40717] test
 1 file changed, 1 deletion(-)
Counting objects: 3, done.
Writing objects: 100% (3/3), 255 bytes | 255.00 KiB/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote:
remote: Create a new pull request for 'branch2/test':
remote:   http://localhost:3000/test1/test/compare/master...branch2%2Ftest
remote:
To http://localhost:3000/test1/test.git
   0c5683f..be40717  branch2/test -> branch2/test

Output of a push when an active PR exists:

[branch2/test 453d638] test
 1 file changed, 1 insertion(+)
Counting objects: 3, done.
Writing objects: 100% (3/3), 255 bytes | 255.00 KiB/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote:
remote: Visit the existing pull request:
remote:   http://localhost:3000/test1/test/pulls/6
remote:
To http://localhost:3000/test1/test.git
   be40717..453d638  branch2/test -> branch2/test

JulienTant · 2018-08-15T10:16:47Z

Now i wanna talk about some technical decision i took :

In case of errors, I log but does nothing because I don't want this feature to block anything.
I created 2 internal endpoints, but I though about an option where i could make only one that gave me everything I needed : the PR default branch + the PR if it exists.
About the fork limitation, If you guys think it's better to create the PR in the "base" repository, then maybe it's better to consider to use what i described above to have the url generation out of the cmd/hook.go file

codecov-io · 2018-08-15T10:30:28Z

Codecov Report

Merging #4716 into master will decrease coverage by 0.02%.
The diff coverage is 28.33%.

@@            Coverage Diff             @@
##           master    #4716      +/-   ##
==========================================
- Coverage    37.4%   37.38%   -0.03%     
==========================================
  Files         307      308       +1     
  Lines       45545    45605      +60     
==========================================
+ Hits        17038    17050      +12     
- Misses      26052    26097      +45     
- Partials     2455     2458       +3

Impacted Files	Coverage Δ
routers/private/internal.go	`62.5% <100%> (+3.4%)`	⬆️
routers/private/repository.go	`25.86% <25.86%> (ø)`
models/repo_indexer.go	`50.84% <0%> (-1.28%)`	⬇️
models/repo_list.go	`62.2% <0%> (-1.17%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cabdf84...d7a6db3. Read the comment docs.

lunny · 2018-08-15T11:37:43Z

gitea_no_gcc should be removed.
I think the PR should be in base repository.

lunny · 2018-08-15T11:38:30Z

And you also should check if user has permission to visit and create pull request.

lafriks · 2018-08-15T11:53:49Z

I also agree that PR should be for base repository as that is default behaviour from UI

JulienTant · 2018-08-15T15:48:42Z

Ok I will first add the fork thing.

And you also should check if user has permission to visit and create pull request.

I'm not sure how to do that. I took a look at the "web" router to access to PR creation page and I found this :

gitea/routers/repo/issue.go

Lines 79 to 91 in 7781e8c

    
           // MustAllowPulls check if repository enable pull requests 
        
           func MustAllowPulls(ctx *context.Context) { 
        
           	if !ctx.Repo.Repository.AllowsPulls() { 
        
           		ctx.NotFound("MustAllowPulls", nil) 
        
           		return 
        
           	} 
        
           	// User can send pull request if owns a forked repository. 
        
           	if ctx.IsSigned && ctx.User.HasForkedRepo(ctx.Repo.Repository.ID) { 
        
           		ctx.Repo.PullRequest.Allowed = true 
        
           		ctx.Repo.PullRequest.HeadInfo = ctx.User.Name + ":" + ctx.Repo.BranchName 
        
           	} 
        
           }

Is that what I must us implement ?

… allowed

JulienTant · 2018-08-15T16:40:33Z

Forks are now created for the base repository.
Ensure that the "target" repository has enabled PR.

lunny · 2018-08-16T01:22:50Z

@@ -0,0 +1,64 @@
+package private


comment head

I changed my PHPStorm template for the project, thanks.

JulienTant · 2018-08-22T15:30:42Z

Hi guys, any news on this ?

lafriks · 2018-08-24T06:16:49Z

@JulienTant it looks good, nice work but I want to test it first before I give my approval

JulienTant · 2018-08-24T08:01:48Z

Of course. I tried to lot of cases but if you find anything i'll be happy to fix/change things.

JulienTant · 2018-10-01T12:41:56Z

up?

lafriks · 2018-10-01T17:55:00Z

We are feature freeze currently so need to wait until v1.6.0-rc1 is released

bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 15, 2018

JulienTant force-pushed the create-pr-after-push branch 3 times, most recently from 00520d7 to 37822e0 Compare August 15, 2018 10:24

JulienTant force-pushed the create-pr-after-push branch from 37822e0 to 0fce8e1 Compare August 15, 2018 14:37

Give user a link to create PR after push

965aa2a

JulienTant force-pushed the create-pr-after-push branch from 0fce8e1 to 965aa2a Compare August 15, 2018 14:41

Forks now create PR in the base repository + make sure PR creation is…

5af0426

… allowed

lafriks added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 15, 2018

lafriks added this to the 1.6.0 milestone Aug 15, 2018

lunny requested changes Aug 16, 2018

View reviewed changes

fix code style

17c233c

techknowlogick modified the milestones: 1.6.0, 1.7.0 Sep 3, 2018

Merge branch 'master' into create-pr-after-push

92d2982

Merge branch 'master' into create-pr-after-push

d7a6db3

lunny approved these changes Oct 20, 2018

View reviewed changes

bkcsoft removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 20, 2018

bkcsoft added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Oct 20, 2018

lunny added the release/highlight Marks a PR as a highlight-worthy change for the release notes. label Oct 20, 2018

lafriks approved these changes Oct 20, 2018

View reviewed changes

bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 20, 2018

lafriks merged commit dea3d84 into go-gitea:master Oct 20, 2018

luxifr mentioned this pull request Jan 22, 2019

Make Pull Request message from PR #4716 configurable #5801

Closed

morph027 mentioned this pull request Aug 28, 2019

link to create PR after push is missing after update from 1.8 to 1.9 on existing repos #8013

Closed

7 tasks

go-gitea locked and limited conversation to collaborators Nov 24, 2020

delvh removed the release/highlight Marks a PR as a highlight-worthy change for the release notes. label Oct 7, 2023

Uh oh!

Conversation

JulienTant commented Aug 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Feature

Output of a push when no active PR exists:

Output of a push when an active PR exists:

Uh oh!

JulienTant commented Aug 15, 2018

Uh oh!

codecov-io commented Aug 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lunny commented Aug 15, 2018

Uh oh!

lunny commented Aug 15, 2018

Uh oh!

lafriks commented Aug 15, 2018

Uh oh!

JulienTant commented Aug 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JulienTant commented Aug 15, 2018

Uh oh!

lunny Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

JulienTant Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

JulienTant commented Aug 22, 2018

Uh oh!

lafriks commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JulienTant commented Aug 24, 2018

Uh oh!

JulienTant commented Oct 1, 2018

Uh oh!

lafriks commented Oct 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

JulienTant commented Aug 15, 2018 •

edited

Loading

codecov-io commented Aug 15, 2018 •

edited

Loading

JulienTant commented Aug 15, 2018 •

edited

Loading

lafriks commented Aug 24, 2018 •

edited

Loading